Add checkout-override-mode option#4010
Conversation
Add setCheckoutEnv helper to export checkout-related vars when no-checkout-override=false
…heckout-override-in-agent-config
# Conflicts: # clicommand/env_set.go # clicommand/env_unset.go
Add skip checkout tests for `no-checkout-override`
Mirror checkout mode affects the checkout but stayed mutable from hooks, plugins, the Job API, and secrets while the lock was on. Also add a disjointness test for the two protection maps and correct the scope test's protection assertions.
AgentStartConfig and BootstrapConfig use inverted command-eval conventions, so their zero values mean opposite things; per-type tables make that legible and drop the kind/switch discriminator.
![buildsworth-bk-app[bot]](https://avatars.githubusercontent.com/in/3010600?s=60&v=4){kind=small}
There was a problem hiding this comment.
I didn’t find any new issues in this pass. This is security-sensitive agent checkout/env protection logic, so it’s above my approval ceiling and I’m leaving the review as a comment.
Want to dig deeper?
The full session log is attached to this Buildkite build. Download the session file and open a new pi session with it:
Download the buildsworth logs from build 5323, then answer my questions about the findings.
from-job was intended to match the agent's historical behaviour, which blocked the backend job env (pipeline/step env) from setting checkout-scoped vars. It was instead letting backend job env override them, reopening the git-flag injection vector the mode exists to close. Lock the backend job env in every mode except none (the same rule secrets follow); only within-job sources (hooks, plugins, Job API) keep the strict-only lock.
There was a problem hiding this comment.
I found one blocking issue around allowed secret mappings not updating the checkout config, and left a question on the default timeout behavior.
Want to dig deeper?
The full session log is attached to this Buildkite build. Download the session file and open a new pi session with it:
Download the buildsworth logs from build 5326, then answer my questions about the findings.
Secrets are written to e.shell.Env in setUp before any hook runs, but the checkout phase reads ExecutorConfig struct fields (GitCloneFlags, SkipCheckout, GitSparseCheckoutPaths) that are only refreshed from env when a hook triggers applyEnvironmentChanges. With no environment/plugin hook, a checkout-scoped var a secret is allowed to set under none mode never reached checkout. Refresh the config right after fetchAndSetSecrets so it takes effect, calling ReadFromEnvironment directly to avoid logging the value.
The checkout-override lock force-emitted the submodules, skip-checkout, skip-fetch, and checkout-timeout vars for both strict and from-job, so under the default from-job mode a pipeline/step env value on the agent's silent side was overwritten with the agent default. For the checkout timeout that meant an unset agent config forced 0 (no timeout), a backward-incompatible change to the default for a value that isn't a git-flag injection vector. Restrict the unconditional force-emit to strict; from-job and none keep the historical conditional emit. The checkout flags stay agent-authoritative under from-job as before. Add from-job silent-side coverage and update the mode docs.
Previous concerns have been addressed.
There was a problem hiding this comment.
The previous secret-refresh and timeout concerns are addressed. I found one new blocker in the secret refresh path.
This is security-sensitive agent checkout/env protection logic, so it remains above my approval ceiling.
Want to dig deeper?
The full session log is attached to this Buildkite build. Download the session file and open a new pi session with it:
Download the buildsworth logs from build 5331, then answer my questions about the findings.
ReadFromEnvironment runs over the full shell env, which now includes secret-backed values (setUp refreshes config right after fetching secrets). The bool/int/slice parse-error branches logged the raw value via the stdlib logger, which bypasses the shell redactors, so a malformed secret-backed value (e.g. an unparseable BUILDKITE_GIT_CHECKOUT_TIMEOUT, or BUILDKITE_GIT_LFS_ENABLED which any source may set) could leak to the job log. Log only the var name (and type) in those warnings.
Previous concerns have been addressed.
There was a problem hiding this comment.
The previous secret-refresh leak is fixed. I left one non-blocking edge case around secret-backed LFS enabling; this security-sensitive checkout/env protection change is still above my approval ceiling, so I’m leaving a comment.
Want to dig deeper?
The full session log is attached to this Buildkite build. Download the session file and open a new pi session with it:
Download the buildsworth logs from build 5332, then answer my questions about the findings.
GIT_LFS_SKIP_SMUDGE was set in setUp before secrets were fetched and the executor config refreshed, so a secret enabling BUILDKITE_GIT_LFS_ENABLED (it's outside both protection maps, so any source may set it) turned on the LFS checkout path without skip-smudge. The plain git checkout would then pull LFS objects through the automatic smudge filter instead of the controlled LFS fetch/checkout path, bypassing sparse scoping. Move the skip-smudge decision to after the secret refresh so it sees the secret-enabled LFS.
There was a problem hiding this comment.
I found one help-text mismatch in the new mode description and left a related LFS timing follow-up in the existing thread. This remains security-sensitive agent checkout/env protection logic, so it’s above my approval ceiling.
Want to dig deeper?
The full session log is attached to this Buildkite build. Download the session file and open a new pi session with it:
Download the buildsworth logs from build 5333, then answer my questions about the findings.
The default from-job mode blocks secrets and keeps the checkout flags authoritative over pipeline/step env, but still lets pipeline/step env set the checkout timeout, submodules, skip-checkout, and skip-fetch-existing-commits toggles the agent leaves unset (matching earlier agent behaviour). Spell out that carve-out so the default isn't read as a complete backend-env lock.
GIT_LFS_SKIP_SMUDGE is set in setUp, but a hook, plugin, or Job API call that enables BUILDKITE_GIT_LFS_ENABLED afterward flips GitLFSEnabled via ReadFromEnvironment without re-applying skip-smudge, so the plain checkout would pull LFS objects through the automatic smudge filter instead of the controlled fetch/checkout path. Re-apply skip-smudge in applyEnvironmentChanges when LFS is enabled, covering within-job sources; setUp still handles the agent-config and secret paths.
There was a problem hiding this comment.
The previous LFS timing and help-text concerns are addressed. I didn’t find any new issues in this pass; this is still security-sensitive agent checkout/env protection logic, so it’s above my approval ceiling.
Want to dig deeper?
The full session log is attached to this Buildkite build. Download the session file and open a new pi session with it:
Download the buildsworth logs from build 5335, then answer my questions about the findings.
BUILDKITE_GIT_COMMIT_VERIFICATION was fully protected, so a pipeline's checkout.commit_verification was silently dropped with an "Ignored" warning even under --checkout-override-mode=none. The backend exposes it under checkout: alongside the flag vars that the mode already governs, so move it from protectedEnv to checkoutOverrideScope. Under none the job env now sets it; from-job and strict keep the agent authoritative.
There was a problem hiding this comment.
I found one blocker in the commit-verification scoping: an allowed post-bootstrap source can set the env var without the executor field being refreshed, so checkout can run with the old value.
Want to dig deeper?
The full session log is attached to this Buildkite build. Download the session file and open a new pi session with it:
Download the buildsworth logs from build 5657, then answer my questions about the findings.
The post-secrets config refresh consumed the env-change detection for config vars like BUILDKITE_GIT_SSH_KEY, so the redacted env-change log stopped firing and TestSecretsIntegration_ConfigVarSecretNotLoggedByEnvChange failed. Rather than re-emit the value and rely on the redactor scrubbing it (the escaping-miss failure mode being hardened against), apply the secret to config silently. It's already announced by name, so the value never needs to reach the log. Strengthen the test to assert the value is never echoed.
Scoping BUILDKITE_GIT_COMMIT_VERIFICATION to the checkout-override mode let secrets and within-job sources set it, but ExecutorConfig.GitCommitVerification had no env tag, so ReadFromEnvironment never refreshed the field. verifyCommit reads the field directly, so checkout could run with the stale value and silently skip the requested verification. Add the env tag so the field is refreshed like the other checkout-scoped vars, and cover the refresh/lock behavior.
Previous concerns have been addressed.
There was a problem hiding this comment.
The previous commit-verification scoping concern is addressed. I didn’t find any new issues in this pass; this remains security-sensitive agent checkout/env protection logic, so it’s above my approval ceiling.
Want to dig deeper?
The full session log is attached to this Buildkite build. Download the session file and open a new pi session with it:
Download the buildsworth logs from build 5662, then answer my questions about the findings.
A step's checkout.sparse.paths arrives as BUILDKITE_GIT_SPARSE_CHECKOUT_PATHS in the backend job env, which createEnvironment locked against every mode except none. Under the default from-job mode the agent's own (empty) sparse config overwrote it, so the feature was unusable without setting checkout-override-mode to none. Sparse paths are passed as argv to `git sparse-checkout set --cone`, not word-split into a git command line, so they can't bypass no-command-eval the way the flag vars can. Give them a from-job floor via IsCheckoutLockedForJobEnv so the backend may set them under the default mode; only strict now locks them. The flag vars and commit_verification keep the none floor.
There was a problem hiding this comment.
I found one non-blocking help-text follow-up in this pass. This remains security-sensitive agent checkout/env protection logic, so it’s above my approval ceiling and I’m leaving the review as a comment.
Want to dig deeper?
The full session log is attached to this Buildkite build. Download the session file and open a new pi session with it:
Download the buildsworth logs from build 6001, then answer my questions about the findings.
The from-job help text enumerated only the timeout/submodules/skip-checkout/ skip-fetch carve-outs, but sparse-checkout paths now also have a from-job floor, so pipeline/step env can override them under the default mode too. Mention it so the help reflects the actual default-mode behaviour.
There was a problem hiding this comment.
I didn’t find any new issues in this pass. This is security-sensitive agent checkout/env protection logic, so it remains above my approval ceiling and I’m leaving the review as a comment.
Want to dig deeper?
The full session log is attached to this Buildkite build. Download the session file and open a new pi session with it:
Download the buildsworth logs from build 6003, then answer my questions about the findings.
5 participants
Description
The agent exposes a number of checkout-related environment variables (git clone/fetch/checkout/clean flags, sparse-checkout paths, submodules, skip-fetch-existing-commits, skip-checkout, commit verification). Historically the protections around these were implicit and inconsistent: some were silently overwritten by agent config in
createEnvironment, others were overridable from the backend job env, hooks, plugins, the Job API, or secrets, with the rules spread across a singleprotectedEnvmap and scattered logic. Because git flags are a shell-injection vector (for example--upload-pack,-c core.sshCommand,ext::transports), letting a job set them can be equivalent to arbitrary command execution and a way to bypassno-command-eval.This PR replaces that implicit handling with an explicit, tri-state
--checkout-override-modeflag (envBUILDKITE_CHECKOUT_OVERRIDE_MODE), shared byagent startandbootstrap, controlling which sources may override the agent's checkout-scoped configuration:strict— the agent is authoritative against every source: pipeline/step env, secrets, hooks, plugins, and the Job API.from-job(default) — hooks, plugins, and the Job API may set checkout-scoped vars (so a job can still tailor its own checkout), but the backend job env (pipeline/step env) and secrets may not. This matches the agent's historical behavior.none— any source, including the backend job env and secrets, may set them.Disabling command evaluation via
no-command-eval=trueforces the mode tostrictregardless of the configured value, so git flags can never be used to undermineno-command-eval.The protection model lives in
env/protected.goand is split into two disjoint tiers (disjointness enforced by a test):protectedEnv— always agent-authoritative in every mode: agent infrastructure, all mirror configuration (BUILDKITE_GIT_MIRRORS_PATH,BUILDKITE_GIT_MIRRORS_LOCK_TIMEOUT,BUILDKITE_GIT_MIRRORS_SKIP_UPDATE,BUILDKITE_GIT_MIRROR_CHECKOUT_MODE,BUILDKITE_GIT_CLONE_MIRROR_FLAGS), andBUILDKITE_GIT_SUBMODULE_CLONE_CONFIG.checkoutOverrideScope— the checkout-scoped vars whose lock depends on the mode:BUILDKITE_GIT_CHECKOUT_FLAGS,BUILDKITE_GIT_CHECKOUT_TIMEOUT,BUILDKITE_GIT_CLEAN_FLAGS,BUILDKITE_GIT_CLONE_FLAGS,BUILDKITE_GIT_COMMIT_VERIFICATION,BUILDKITE_GIT_FETCH_FLAGS,BUILDKITE_GIT_SKIP_FETCH_EXISTING_COMMITS,BUILDKITE_GIT_SPARSE_CHECKOUT_PATHS,BUILDKITE_GIT_SUBMODULES,BUILDKITE_SKIP_CHECKOUT.BUILDKITE_GIT_COMMIT_VERIFICATIONis an enum (""/warn/strict), not an injection vector, but the backend exposes it undercheckout:alongside the flag vars, so it is governed by the mode too.Context
Linear
Changes
New CLI flag, shared by
agent startandbootstrap:CheckoutOverrideModetype inenv(strict/from-job/none) withParseCheckoutOverrideMode,String, andRestrictedForCommandEval(forcesstrictwhen command-eval is off).AgentConfiguration,AgentStartConfig,BootstrapConfig,ExecutorConfig, and the Job API server (WithCheckoutOverrideMode), via a sharedresolveCheckoutOverrideModehelper that applies the command-eval floor.ExecutorConfig.CheckoutOverrideModehas noenvtag, so a hook cannot relax the mode mid-job.env/protected.gointoprotectedEnv(always protected) andcheckoutOverrideScope(mode-dependent), and adds predicatesIsCheckoutOverrideScoped,IsCheckoutLocked(within-job sources: hooks, plugins, and the Job API;strictonly), andIsCheckoutLockedForSecrets(secrets; every mode exceptnone). The backend job env follows the same rule as secrets (locked in every mode exceptnone), enforced increateEnvironment. The two maps are asserted disjoint.BUILDKITE_GIT_COMMIT_VERIFICATIONto the mode: it was fully protected, so a pipeline'scheckout.commit_verificationwas silently dropped with an "Ignored" warning even under--checkout-override-mode=none. Because the backend exposes it undercheckout:alongside the flag vars, it moves fromprotectedEnvtocheckoutOverrideScopeand is emitted viasetCheckoutEnvincreateEnvironment. Undernonethe job env now sets it (so verification can actually run);from-jobandstrictkeep the agent authoritative.createEnvironment(setCheckoutEnv), runtime config refresh inReadFromEnvironment, hook env changes inapplyEnvironmentChanges, secret-to-env mappings infetchAndSetSecrets, and Job API patch/delete (checkProtected).BUILDKITE_GIT_SUBMODULE_CLONE_CONFIGalways agent-authoritative, including movingBUILDKITE_GIT_MIRRORS_SKIP_UPDATEandBUILDKITE_GIT_CLONE_MIRROR_FLAGSout of the mode-dependent scope (the mirror is shared infrastructure and clone-mirror flags are a cross-job injection vector).env,internal/jobconfig, theagentjob runner, and the hooks/plugins/secrets/Job API paths, including: the job-env override matrix across all three modes, infra vars staying agent-authoritative even undernone, secrets rejected for the protected checkout-infra vars, commit verification following the mode (enabled from job env undernone, locked underfrom-job/strict), and an end-to-end test that disabling command-eval auto-forcesstrict.Agent behavior changes
Under
from-job(the default), checkout-scoped vars remain overridable from within-job sources (hooks, plugins, and the Job API) but not from the backend job env or secrets, matching the agent's historical behavior.strictoffers a more protected behavior: hooks and plugins can no longer set the checkout-scoped or mirror vars (on main several were mutable from within the job).noneis the only mode that opens checkout-scoped vars to the backend job env and secrets. As part of this,BUILDKITE_GIT_COMMIT_VERIFICATIONis now mode-governed rather than always dropped: a pipeline'scheckout.commit_verificationtakes effect undernone, whilefrom-jobandstrictkeep the agent authoritative.Testing
go test ./...).go tool gofumpt -extra -w .)go test ./...passes locally, including the full checkout-override-mode unit and integration coverage acrossenv,internal/job,agent,jobapi, andclicommand, plus the commit-verification cases added toagent/integration/job_environment_integration_test.go.Disclosures / Credits
Code generated by Claude Code (Opus 4.8) under my guidance. Multiple reviews were conducted to establish the correctness and completeness of the implementation of this new agent config option and protection model. The core feature and its conversion from the earlier binary design to the tri-state mode were implemented in earlier revisions.